From ce5a29bc384542839a5f12061499c8ec706b1c34 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Fri, 22 Oct 2010 16:12:16 +0100 Subject: [PATCH] recent-manager: Coalesce multiple changes MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Since the ::changed implementation of GtkRecentManager implies a synchronous write operation, when we receive multiple requests to emit a ::changed signal we might end up blocking. This change coalesces multiple ::changed emission requests using the following sequence: • the first request will install a timeout in 250 ms, which will emit the ::changed signal • each further request while the timeout has not been emitted will increase a counter ‣ if the counter reaches 250 before the timeout has been emitted, then the RecentManager will remove the timeout source and force a signal emission and reset the counter This sequence should guarantee that frequent ::changed emission requests are coalesced, and also guarantee that we don't let them dangle for too long. https://bugzilla.gnome.org/show_bug.cgi?id=616997 --- gtk/gtkrecentmanager.c | 99 ++++++++++++++++++++++++++++----------- gtk/tests/recentmanager.c | 86 ++++++++++++++++++++++++++++------ 2 files changed, 143 insertions(+), 42 deletions(-) diff --git a/gtk/gtkrecentmanager.c b/gtk/gtkrecentmanager.c index 9730172ba9..ccdea45310 100644 --- a/gtk/gtkrecentmanager.c +++ b/gtk/gtkrecentmanager.c @@ -176,6 +176,9 @@ struct _GtkRecentManagerPrivate GBookmarkFile *recent_items; GFileMonitor *monitor; + + guint changed_timeout; + guint changed_age; }; enum @@ -390,12 +393,26 @@ gtk_recent_manager_get_property (GObject *object, } static void -gtk_recent_manager_dispose (GObject *object) +gtk_recent_manager_finalize (GObject *object) { GtkRecentManager *manager = GTK_RECENT_MANAGER (object); GtkRecentManagerPrivate *priv = manager->priv; - if (priv->monitor) + g_free (priv->filename); + + if (priv->recent_items != NULL) + g_bookmark_file_free (priv->recent_items); + + G_OBJECT_CLASS (gtk_recent_manager_parent_class)->finalize (object); +} + +static void +gtk_recent_manager_dispose (GObject *gobject) +{ + GtkRecentManager *manager = GTK_RECENT_MANAGER (gobject); + GtkRecentManagerPrivate *priv = manager->priv; + + if (priv->monitor != NULL) { g_signal_handlers_disconnect_by_func (priv->monitor, G_CALLBACK (gtk_recent_manager_monitor_changed), @@ -404,21 +421,21 @@ gtk_recent_manager_dispose (GObject *object) priv->monitor = NULL; } - G_OBJECT_CLASS (gtk_recent_manager_parent_class)->dispose (object); -} - -static void -gtk_recent_manager_finalize (GObject *object) -{ - GtkRecentManager *manager = GTK_RECENT_MANAGER (object); - GtkRecentManagerPrivate *priv = manager->priv; + if (priv->changed_timeout != 0) + { + g_source_remove (priv->changed_timeout); + priv->changed_timeout = 0; + priv->changed_age = 0; + } - g_free (priv->filename); - - if (priv->recent_items) - g_bookmark_file_free (priv->recent_items); + if (priv->is_dirty) + { + g_object_ref (manager); + g_signal_emit (manager, signal_changed, 0); + g_object_unref (manager); + } - G_OBJECT_CLASS (gtk_recent_manager_parent_class)->finalize (object); + G_OBJECT_CLASS (gtk_recent_manager_parent_class)->dispose (gobject); } static void @@ -456,8 +473,6 @@ gtk_recent_manager_real_changed (GtkRecentManager *manager) else if (age == 0) { g_bookmark_file_free (priv->recent_items); - priv->recent_items = NULL; - priv->recent_items = g_bookmark_file_new (); } } @@ -942,7 +957,6 @@ gtk_recent_manager_add_full (GtkRecentManager *manager, * will dump our changes */ priv->is_dirty = TRUE; - gtk_recent_manager_changed (manager); return TRUE; @@ -1003,7 +1017,6 @@ gtk_recent_manager_remove_item (GtkRecentManager *manager, } priv->is_dirty = TRUE; - gtk_recent_manager_changed (manager); return TRUE; @@ -1227,7 +1240,6 @@ gtk_recent_manager_move_item (GtkRecentManager *recent_manager, } priv->is_dirty = TRUE; - gtk_recent_manager_changed (recent_manager); return TRUE; @@ -1282,17 +1294,15 @@ purge_recent_items_list (GtkRecentManager *manager, { GtkRecentManagerPrivate *priv = manager->priv; - if (!priv->recent_items) + if (priv->recent_items == NULL) return; - + g_bookmark_file_free (priv->recent_items); - priv->recent_items = NULL; - priv->recent_items = g_bookmark_file_new (); priv->size = 0; - priv->is_dirty = TRUE; - + /* emit the changed signal, to ensure that the purge is written */ + priv->is_dirty = TRUE; gtk_recent_manager_changed (manager); } @@ -1332,10 +1342,43 @@ gtk_recent_manager_purge_items (GtkRecentManager *manager, return purged; } +static gboolean +emit_manager_changed (gpointer data) +{ + GtkRecentManager *manager = data; + + manager->priv->changed_age = 0; + manager->priv->changed_timeout = 0; + + g_signal_emit (manager, signal_changed, 0); + + return FALSE; +} + static void -gtk_recent_manager_changed (GtkRecentManager *recent_manager) +gtk_recent_manager_changed (GtkRecentManager *manager) { - g_signal_emit (recent_manager, signal_changed, 0); + /* coalesce consecutive changes + * + * we schedule a write in 250 msecs immediately; if we get more than one + * request per millisecond before the timeout has a chance to run, we + * schedule an emission immediately. + */ + if (manager->priv->changed_timeout == 0) + manager->priv->changed_timeout = gdk_threads_add_timeout (250, emit_manager_changed, manager); + else + { + manager->priv->changed_age += 1; + + if (manager->priv->changed_age > 250) + { + g_source_remove (manager->priv->changed_timeout); + g_signal_emit (manager, signal_changed, 0); + + manager->priv->changed_age = 0; + manager->priv->changed_timeout = 0; + } + } } static void diff --git a/gtk/tests/recentmanager.c b/gtk/tests/recentmanager.c index 6f0fa5839b..7f8f95b3e1 100644 --- a/gtk/tests/recentmanager.c +++ b/gtk/tests/recentmanager.c @@ -19,6 +19,7 @@ * Boston, MA 02111-1307, USA. */ +#include #include const gchar *uri = "file:///tmp/testrecentchooser.txt"; @@ -95,6 +96,69 @@ recent_manager_add (void) g_slice_free (GtkRecentData, recent_data); } +typedef struct { + GMainLoop *main_loop; + gint counter; +} AddManyClosure; + +static void +check_bulk (GtkRecentManager *manager, + gpointer data) +{ + AddManyClosure *closure = data; + + if (g_test_verbose ()) + g_print (G_STRLOC ": counter = %d\n", closure->counter); + + g_assert_cmpint (closure->counter, ==, 100); + + if (g_main_loop_is_running (closure->main_loop)) + g_main_loop_quit (closure->main_loop); +} + +static void +recent_manager_add_many (void) +{ + GtkRecentManager *manager = g_object_new (GTK_TYPE_RECENT_MANAGER, + "filename", "recently-used.xbel", + NULL); + AddManyClosure *closure = g_new (AddManyClosure, 1); + GtkRecentData *data = g_slice_new0 (GtkRecentData); + gint i; + + closure->main_loop = g_main_loop_new (NULL, FALSE); + closure->counter = 0; + + g_signal_connect (manager, "changed", G_CALLBACK (check_bulk), closure); + + for (i = 0; i < 100; i++) + { + gchar *new_uri; + + data->mime_type = "text/plain"; + data->app_name = "testrecentchooser"; + data->app_exec = "testrecentchooser %u"; + + if (g_test_verbose ()) + g_print (G_STRLOC ": adding item %d\n", i); + + new_uri = g_strdup_printf ("file:///doesnotexist-%d.txt", i); + gtk_recent_manager_add_full (manager, new_uri, data); + g_free (new_uri); + + closure->counter += 1; + } + + g_main_loop_run (closure->main_loop); + + g_main_loop_unref (closure->main_loop); + g_slice_free (GtkRecentData, data); + g_free (closure); + g_object_unref (manager); + + g_assert_cmpint (g_unlink ("recently-used.xbel"), ==, 0); +} + static void recent_manager_has_item (void) { @@ -234,20 +298,14 @@ main (int argc, { gtk_test_init (&argc, &argv, NULL); - g_test_add_func ("/recent-manager/get-default", - recent_manager_get_default); - g_test_add_func ("/recent-manager/add", - recent_manager_add); - g_test_add_func ("/recent-manager/has-item", - recent_manager_has_item); - g_test_add_func ("/recent-manager/move-item", - recent_manager_move_item); - g_test_add_func ("/recent-manager/lookup-item", - recent_manager_lookup_item); - g_test_add_func ("/recent-manager/remove-item", - recent_manager_remove_item); - g_test_add_func ("/recent-manager/purge", - recent_manager_purge); + g_test_add_func ("/recent-manager/get-default", recent_manager_get_default); + g_test_add_func ("/recent-manager/add", recent_manager_add); + g_test_add_func ("/recent-manager/add-many", recent_manager_add_many); + g_test_add_func ("/recent-manager/has-item", recent_manager_has_item); + g_test_add_func ("/recent-manager/move-item", recent_manager_move_item); + g_test_add_func ("/recent-manager/lookup-item", recent_manager_lookup_item); + g_test_add_func ("/recent-manager/remove-item", recent_manager_remove_item); + g_test_add_func ("/recent-manager/purge", recent_manager_purge); return g_test_run (); } -- 2.30.2